Repository navigation
feat(bigquery-storage): add Arrow options and cancel to read_rows - #18643
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for arrow_serialization_options in the BigQuery Storage read_rows client and stream reader, allowing users to specify options like buffer compression. It also implements a cancel() method on ReadRowsStream to stop reading before the end of a stream without reconnecting, along with corresponding unit and system tests. The review feedback highlights a potential race condition in _reconnect() where a newly established stream could be leaked if cancel() is called concurrently, and suggests a code change to safely assign and check the cancellation state.
| if self._cancelled: | ||
| cancel = getattr(wrapped, "cancel", None) | ||
| if cancel is not None: | ||
| cancel() | ||
| return | ||
| self._wrapped = wrapped | ||
| break |
There was a problem hiding this comment.
There is a potential race condition when cancel() is called concurrently from another thread (which is the standard way to cancel a blocking stream iterator) while _reconnect() is establishing a new stream.
If cancel() is called after the self._cancelled check on line 222 but before self._wrapped = wrapped on line 227, the cancel() call will execute on the old stream (or do nothing if it was None). Then, the new stream wrapped is assigned to self._wrapped and the loop breaks. Since self._cancelled is now True, the __iter__ loop will terminate, but the newly created wrapped stream is never cancelled or closed, leading to a resource leak.
To prevent this, assign self._wrapped = wrapped first, and then check self._cancelled. If it is cancelled, set self._wrapped = None (to preserve the expected state) and cancel the stream before returning.
| if self._cancelled: | |
| cancel = getattr(wrapped, "cancel", None) | |
| if cancel is not None: | |
| cancel() | |
| return | |
| self._wrapped = wrapped | |
| break | |
| self._wrapped = wrapped | |
| if self._cancelled: | |
| self._wrapped = None | |
| cancel = getattr(wrapped, "cancel", None) | |
| if cancel is not None: | |
| cancel() | |
| return | |
| break |
There was a problem hiding this comment.
Good catch, the window between the check and the assignment was real. Fixed in f759307: _reconnect() now stores the new call first and then checks _cancelled, so a concurrent cancel() either cancels it directly or gets picked up by the check.
Default job streams (projects/.../jobs/.../streams/_default) accept ArrowSerializationOptions on each ReadRowsRequest, but BigQueryReadClient.read_rows() had no way to send them, and ReadRowsStream dropped them when it reconnected after a transient error. There was also no way to stop a stream early without closing the whole client. read_rows() now takes arrow_serialization_options, and ReadRowsStream sends them on the first request and on every reconnect, so all batches in a stream use the same encoding. ReadRowsStream.cancel() cancels the current call and stops further reconnects, including a reconnect already in progress, without closing the client. google-cloud-bigquery needs both to read query results in Arrow format through the default job stream.
f759307 to
ea74a33
Compare
| offset, | ||
| read_rows_kwargs, | ||
| retry_delay_callback=None, | ||
| arrow_serialization_options=None, |
There was a problem hiding this comment.
Why aren't you using the existing read_rows_kwargs dictionary?
Edit: I see that client is the super() / generated client. Still, I'd prefer adding a more general "request_template" or something like that so that we can pass any new request parameters more easily in future.
There was a problem hiding this comment.
replaced arrow_serialization_options on ReadRowsStream.__init__ with *, request_template=None, and copied it via gapic_types.ReadRowsRequest(self._request_template) in _reconnect() before setting read_stream and offset so caller-supplied templates aren't mutated across reconnects.
|
|
||
| self._reconnect() | ||
|
|
||
| def cancel(self): |
There was a problem hiding this comment.
Thought: should we automatically call cancel in __del__? That might help in cases where folks are just reading one batch and returning.
There was a problem hiding this comment.
added __del__ to call self.cancel() when not getattr(self, "_cancelled", True), and cleared self._wrapped = None when iteration completes naturally so finished or already-cancelled streams aren't cancelled again on GC.
…cancel in __del__ Require keyword argument syntax for arrow_serialization_options on BigQueryReadClient.read_rows() and ReadRowsStream.__init__(). Cancel unfinished streams automatically in ReadRowsStream.__del__(), and clear _wrapped upon normal stream completion so completed or already-cancelled streams are not cancelled again during garbage collection.
Replace arrow_serialization_options on ReadRowsStream.__init__ with a keyword-only request_template parameter so future ReadRowsRequest options can be passed without changing the stream constructor. Copy the template via ReadRowsRequest(self._request_template) on reconnect to avoid mutating caller-supplied request objects.
| offset=self._offset, | ||
| **self._read_rows_kwargs, | ||
| ) | ||
| if self._request_template is None: |
There was a problem hiding this comment.
Nit: if we do this check in __init__ it would simplify this loop.
There was a problem hiding this comment.
I looked at moving this into __init__, but the v1beta2 client reuses this reader with its own GAPIC client. When there's no template, the flattened read_stream/offset call is what keeps it working, since passing a v1 ReadRowsRequest raises TypeError there. So I've left the branch as is.
If you're not convinced, I'm happy to follow up on this.
🤖 I have created a release *beep* *boop* --- <details><summary>db-dtypes: 1.8.0</summary> ## [1.8.0](db-dtypes-v1.7.2...db-dtypes-v1.8.0) (2026-10-15) ### Features * **db-dtypes:** support pandas 3.1 ([#18590](#18590)) ([f18a11e](f18a11e)) </details> <details><summary>gapic-generator: 1.43.1</summary> ## [1.43.1](gapic-generator-v1.43.0...gapic-generator-v1.43.1) (2026-10-15) ### Bug Fixes * **gapic-generator:** do not emit empty routing metadata on client-streaming RPCs ([#18591](#18591)) ([bbdd643](bbdd643)) * **gapic-generator:** resolve showcase-mypy incompatible redefinition ([7032a89](7032a89)) * **gapic-generator:** resolve showcase-mypy incompatible redefinition and add canary CI job ([#18701](#18701)) ([7032a89](7032a89)) </details> <details><summary>gcp-sphinx-docfx-yaml: 3.4.0</summary> ## [3.4.0](gcp-sphinx-docfx-yaml-v3.3.3...gcp-sphinx-docfx-yaml-v3.4.0) (2026-10-15) ### Features * **gcp-sphinx-docfx-yaml:** optimize performance ([#18588](#18588)) ([cf1a0e5](cf1a0e5)) </details> <details><summary>google-cloud-alloydb: 0.11.4</summary> ## [0.11.4](google-cloud-alloydb-v0.11.3...google-cloud-alloydb-v0.11.4) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-bigquery: 3.46.2</summary> ## [3.46.2](google-cloud-bigquery-v3.46.1...google-cloud-bigquery-v3.46.2) (2026-10-15) ### Bug Fixes * **bigquery:** accept empty sourceFormat fields ([#18472](#18472)) ([590e43d](590e43d)) * **bigquery:** drop startIndex whenever a page token is sent ([#18603](#18603)) ([fc6eae3](fc6eae3)) * **bigquery:** remove PendingDeprecationWarning from RowIterator.to_arrow ([#18684](#18684)) ([9025061](9025061)) </details> <details><summary>google-cloud-bigquery-storage: 2.43.0</summary> ## [2.43.0](google-cloud-bigquery-storage-v2.42.0...google-cloud-bigquery-storage-v2.43.0) (2026-10-15) ### Features * **bigquery-storage:** add Arrow options and cancel to read_rows ([#18643](#18643)) ([939bdd0](939bdd0)) </details> <details><summary>google-cloud-bigtable: 2.50.1</summary> ## [2.50.1](google-cloud-bigtable-v2.50.0...google-cloud-bigtable-v2.50.1) (2026-10-15) ### Bug Fixes * **bigtable:** avoid hang when batcher not closed on process exit ([#18610](#18610)) ([7674c91](7674c91)) * **bigtable:** fix authentication scopes for metrics exporter ([#18609](#18609)) ([722d34e](722d34e)) </details> <details><summary>google-cloud-build: 3.41.0</summary> ## [3.41.0](google-cloud-build-v3.40.0...google-cloud-build-v3.41.0) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-compute: 1.56.0</summary> ## [1.56.0](google-cloud-compute-v1.55.0...google-cloud-compute-v1.56.0) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-compute-v1beta: 0.12.9</summary> ## [0.12.9](google-cloud-compute-v1beta-v0.12.8...google-cloud-compute-v1beta-v0.12.9) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-contact-center-insights: 1.29.0</summary> ## [1.29.0](google-cloud-contact-center-insights-v1.28.0...google-cloud-contact-center-insights-v1.29.0) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-dataproc: 5.32.0</summary> ## [5.32.0](google-cloud-dataproc-v5.31.0...google-cloud-dataproc-v5.32.0) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-datastream: 1.21.0</summary> ## [1.21.0](google-cloud-datastream-v1.20.0...google-cloud-datastream-v1.21.0) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-memorystore: 0.5.6</summary> ## [0.5.6](google-cloud-memorystore-v0.5.5...google-cloud-memorystore-v0.5.6) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-pubsub: 2.43.0</summary> ## [2.43.0](google-cloud-pubsub-v2.42.0...google-cloud-pubsub-v2.43.0) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-redis-cluster: 0.5.5</summary> ## [0.5.5](google-cloud-redis-cluster-v0.5.4...google-cloud-redis-cluster-v0.5.5) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-securitycenter: 1.48.0</summary> ## [1.48.0](google-cloud-securitycenter-v1.47.0...google-cloud-securitycenter-v1.48.0) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> <details><summary>google-cloud-storage: 3.17.1</summary> ## [3.17.1](google-cloud-storage-v3.17.0...google-cloud-storage-v3.17.1) (2026-10-15) ### Bug Fixes * **storage:** set GRPC_ALTS_MAX_CONCURRENT_HANDSHAKES=2 in benchmark runner ([#18703](#18703)) ([a9ca134](a9ca134)) </details> <details><summary>google-developer-knowledge: 0.1.5</summary> ## [0.1.5](google-developer-knowledge-v0.1.4...google-developer-knowledge-v0.1.5) (2026-10-15) ### Features * update API sources and regenerate ([#18699](#18699)) ([1aa57db](1aa57db)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Adds
arrow_serialization_optionstoBigQueryReadClient.read_rows()and aReadRowsStream.cancel()method, for reading Arrow results from a query job's default stream.ReadRowscall, including reconnects.cancel()stops the stream, including during a reconnect, without closing the client.LZ4_FRAME/ZSTDandcancel()on a default job stream.google-cloud-bigquery will depend on the release that includes this.